Conversation
It is just a few lines, but if the code is mostly copy-pasted, then let's be on the safe side: please separate the macro into its own file so that you can use the original licence. Also please add a comment with who has the copyright / where the code is taken from.
Please take a look at |
rust/kernel/print.rs
Outdated
| // `$val` expression could be a block (`{ .. }`), in which case the `eprintln!` | ||
| // will be malformed. | ||
| () => { | ||
| $crate::pr_debug!("[{}:{}]", core::file!(), core::line!()); |
There was a problem hiding this comment.
Note that pr_* does not add a newline -- did you try it at runtime?
There was a problem hiding this comment.
It seems to not matter, the prints show up in their own line. Maybe its just dmesg being nice though, I can definetely add the newline for good practice.
I messed up a bit: While now testing this again (we used to have it with pr_info back at https://github.com/niklasmohrin/linux/blob/2d803fceab31195ff6fa8ad209b2fe0438f130cf/rust/kernel/print.rs), I saw that printing on the debug level is actually different from the other levels (I found https://stackoverflow.com/questions/28936199/why-is-pr-debug-of-the-linux-kernel-not-giving-any-output). When I copied it over from our branch and saw that there now is a pr_debug, I just replaced it with that thinking it would be more fitting. Now I find pr_debug to actually be rather unconvenient though and would prefer something like pr_info (should have checked again before opening, sorry for that).
If I (as a dev just wanting to print debug info) would have to set up the kernel with some debug flags or settings, I would be more inclined to just use pr_info with mediocre formatting instead of dbg. Maybe I am missing something though ... :)
I guess there could be a CI job that ensures that there are no dbg! calls in production code so that the info level log is not getting "spammed" if that would be a problem. What do you or others think?
There was a problem hiding this comment.
I guess there could be a CI job that ensures that there are no
dbg!calls in production code so that theinfolevel log is not getting "spammed" if that would be a problem. What do you or others think?
Sounds like a good idea -- could you please create an issue to about forgetting about it?
|
(no changes since last time, not done yet) Thanks for your quick response :) Should we use MIT or Apache? Its also hard to point out a copyright holder other than maybe the Rust foundation? Or "the Rust developers"? Or there could just be a link to the github repository and https://rust-lang.org
Will do :) |
SPDX has expressions and you can When you only take code, it's okay to choose one license from multiple licenses; but this means you can no longer contribute back to the original project. Essentially, it's Apache-2.0 is not compatible with GPL-2.0, this means that when the code is being compiled in the Linux kernel, we use the MIT license. But the Apache-2.0 still has to be retained. EDIT: Also, Apache-2.0 is the main license for Rust. MIT is only there for GPL-2.0 compatibility. |
Take a look at how I did it for our in-tree |
niklasmohrin
force-pushed
the
dbg-macro
branch
2 times, most recently
from
2fbefd6 to
dfefab8
Compare
The Rust standard library has a really handy macro, `dbg!`. It prints the source location (filename and line) along with the raw source code that is is invoked with and the `Debug` representation of the given expression. This patch ports the macro over to the kernel crate. For use in the kernel, we don't print with `eprintln!` (stderr), but `pr_info!`. Since the source code for the macro is taken from the standard library source (with only minor adjustments), the designated file (`std_vendor.rs`) is licensed under "Apache 2.0 OR MIT", just like its origin. This is the same approach as with the vendored `alloc` crate. Signed-off-by: Niklas Mohrin <[email protected]>
niklasmohrin
force-pushed
the
dbg-macro
branch
from
dfefab8 to
1611373
Compare
|
I think everything is worked in now |
|
Looks good. It seems I cannot re-run the PR actions before merging... Let's see. |
3 participants
Hey there,
I really like the
dbg!macro from the standard library and thought that it would make a good addition to the existing printing macros (and it was something that could be split off from #459 :D).I am not sure about licensing, since the Rust standard lib is licensed under apache/mit and this file is licensed under gpl. Would that be a problem?
This is my first time contributing to the kernel, so if there is something wrong with commit mesasges or anything else, please point me in the right direction :)